Added all the modified files#310
Conversation
|
vraparla-mbp:crds vraparla$ kubectl get contrailstatusmonitor -n contrail Ran couple of times. |
| listKind: PostgresList | ||
| plural: postgres | ||
| singular: postgres | ||
| scope: Namespaced |
There was a problem hiding this comment.
Please update operator-sdk to version v0.17.1 as described in documentation.
There was a problem hiding this comment.
As per document, for 2008R we are using the older version, I have noticed u have changes done in master not in 2008R.
| commonConfiguration: | ||
| activate: true | ||
| create: true | ||
| activate: false |
There was a problem hiding this comment.
Why is this changed from true to false?
| // Note: | ||
| // The Controller will requeue the Request to be processed again if the returned error is non-nil or | ||
| // Result.Requeue is true, otherwise upon completion it will remove the work from the queue. | ||
| func (r *ReconcileContrailmonitor) Reconcile(request reconcile.Request) (reconcile.Result, error) { |
There was a problem hiding this comment.
I don't see any unit tests for that controller. As far as I know there is a requirement that a new code should be covered by unit tests
There was a problem hiding this comment.
Moreover I strongly suggest to extend one of the e2e tests that will verify happy path. Just a single simple check
There was a problem hiding this comment.
I have not included unit test or e2e test cases here, I have noticed it, the reason is, I get very inconstant results, One day, I get one results, other day, i get completly diffrent results., I like you to suggest me what is wrong in the code that gives varied results. for example, even if the postgres service is active as it updates my additional print coloums crd instance it updates the field as Not active. it is very random
There was a problem hiding this comment.
If your code works inconsistently you should write unit tests and e2e tests and fix it. That is test driven development.
| if err != nil { | ||
| return reconcile.Result{}, err | ||
| } | ||
| fmt.Println(clist.Items[0].Status.ServiceStatus) |
There was a problem hiding this comment.
It looks like a debug log. Please use logger or remove it
There was a problem hiding this comment.
I will remove it.
| } | ||
| serIns3 := &contrailv1alpha1.Contrailstatusmonitor{ObjectMeta: metav1.ObjectMeta{Name: provisionmanager.Name, Namespace: "contrail"}} | ||
|
|
||
| if provisionmanager.Status.Active == nil { |
There was a problem hiding this comment.
Yes, WE need to report Active/NotActive for postgres, provisionmanager, zookeeper, cassandra etc.
Yes, I have noticed the point that you have raised, These fields in the crds are boolean pointers, I can check boolean pointer as the GCP build raised Panic error, which is not raised by the blazel build, so, after couple of iterations , I have decided with 2 values nil and true, I tried to even check the IsAlive function that cassandra, zookeper, rabbitmq used to to test the pod activity, I noticed they are very inconsiatnat in returning the value.
|
Added my points, I need ur review on few points, My results are very inconsistent on day to day basis. I don't know the reason, Specially, when the CRD updates the status field , even though the work flow captures "active" it reports and updates as "notActive", on another service, it captures correctly. |
2 participants
No description provided.